Codecov Report

Merging #2998 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2998      +/-   ##
==========================================
- Coverage   31.51%   31.49%   -0.02%     
==========================================
  Files         221      221              
  Lines        6305     6309       +4     
  Branches     1121     1122       +1     
==========================================
  Hits         1987     1987              
- Misses       3629     3632       +3     
- Partials      689      690       +1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24f6cb7...591e55e. Read the comment docs.

Copying and pasting the accessibility items from #2960

1
Screen readers, especially the Windows ones (e.g. NVDA, JAWS) work in two different modes: "browse mode" and "forms mode". In "browse mode" they intercept key presses for their own goals and don't pass them to the browser. As a result, browsers are unaware of JS events on non-interactive elements (in our case the BlockToolbar). Only when screen readers switch to "forms mode", e.g. when the users lands on a form field, they pass key presses to the browser. They do the same on non-interactive elements only if they have some special ARIA roles. Only in this case JS events work on non-interactive elements.
In a few words: arrows navigation doesn't work with NVDA and JAWS (and seems it's buggy with Safari/VoiceOver too) because the browser doesn't know a keyboard event happened on the toolbar. To fix this, the toolbar needs an ARIA role="toolbar". Given the current markup, this would be a bit tricky, see below.

2
Toolbars need to have a "flat" structure, just a div with controls inside. Not a list.
I understand this is a bit tricky to address because the toolbars are actually composed by multiple toolbars, each one using a list. I've mentioned this issue a while ago in a conversation with @aduth . This should be addressed, and maybe #2148 would be the proper place.
Update: the toolbars won't be lists any longer if #3001 gets merged. However, there should be just one wrapper with role="toolbar" that contains all the controls. To my understanding, this would be easier with #3001 because the wrapper would contain just divs (and the controls). Will need testing with Windows screen readers.

3
To meet the ARIA spec and the ARIA Authoring Practices, navigation through the toolbar controls should happen with arrow keys only (no Tab). See https://www.w3.org/TR/wai-aria-practices/#toolbar

4
The toolbar should be labeled with an aria-label attribute. Worth noting the role="toolbar" already makes screen readers announce "tool bar" so the aria-label should say just something like "Formatting".

Thank you for working on this. 🏅

I think this is a great start. I think unifying the button style should be done separately, I have on my todo to look at focus styles again, given all that's changed and what we've learned. I have some ideas here, but I think this is probably fine for now.

The permalink should probably live in the Status menu of the sidebar, and if/when we merge the publish dropdown, there as well.

Can we keep the Classic blocks docked toolbar? It works pretty well there, and it is a rather unique block compared to all the others.

I would love to jump in and fix mobile in this one — I hope to add a breakpoint that makes the toolbar stacked below the editor bar — but perhaps it's best I jump in once some of afercia's notes are addressed?

Rebase this to fix the accessibility issues:

• The toolbar has arrow navigation
• The toolbar has in/out shortcuts
• The toolbar has the toolbar role
• The toolbar is a div and no uls anymore
• The toolbar has a label (probably not a great one but "Formatting" is not generic enough)

I will take a look at the responsiveness/mobile on monday. Thank you Riad!

I'm looking at some polish here but for now I'm seeing this JS warning when setting focus on a text block:

@jasmussen yep, this should be fixed in master (just rebase)

I pushed a tweak so that the toolbar stacks on the mobile breakpoint.

But it's not perfect. Long term we should work to lower the breakpoint that causes these to stack, right now they stack already at 960.

Short term we should think about doing some polish to how the "mobile controls" work. Right now the "mobile controls" are basically the mover, and the now-in-dropdown Settings, Delete and HTML mode. These are now in 2 levels of drawers that all look weird.

I see two ways we can fix this:

1. Drop the current implementation we have for handling the mover and trash on mobile, and instead move these to the ellipsis dropdown. Then we can put the ellipsis dropdown in the quick toolbar on mobile.

2. Start thinking about Handling overflow in the quick toolbar #2898, and how that could potentially solve both the issue of the mover/trash/settings, as well as any item overflow in the quick toolbar.

There's also an issue with the fact that the toolbar disappears entirely when nothing is selected. This means the stacked toolbar overlaps the scrollbar. Perhaps worth thinking about showing a grayed text block toolbar when nothing is selected, or something in that vein.

A minor issue with Hello Dolly and smaller screen sizes:

Rebased this and fixed some issues with the mobile toolbar. Do you think we could merge this soon in the weekend to discover any drawbacks before the next release?

I think it would be good to get this in soon, yes. Nice fixes.

I wish the stacked toolbar didn't cover the scrollable area, but instead pushed it down:

However I feel like when I pushed fixes to this branch, I tried addressing that and running into issues. Perhaps that if you have nothing selected, the stacked toolbar disappears entirely:

Perhaps we should embrace the toolbar and show it there, even when empty:

This would allow us to change the scrollable areas top coordinate so it's never covered. I'm also confident that this toolbar area — when no block is selected — we'll soon find a use for. Whether through extensions or permalinks or something else.

I also got this JS error while messing around, not sure if it's this branch or also master:

Try these steps to reproduce:

• Type in a paragraph
• In a new paragraph paste a bunch of text

Or this one:

• Type in a bunch of paragraphs
• Change one of them to a quote
• Delete the quote

I pushed a fix for the JS error 5d16fa3

The toolbar is always visible on mobile now.

Nice. Couple of issues though:

It covers the sidebar

We'll also want to push down this region so the scrollbar starts after the docked toolbar:

Thanks for working on this.

Not easy to get right but I think it's ok now? bb70961

This seems great to me now. And I do agree, would be good to get this in soon so we can both test it, and polish it before the next version. CC: @mtias

@youknowriad is this rebased with master? I'm not seeing the keyboard improvements.

@mtias not the last master, rebasing.

Edit: It is now

Merged to give us time to fix any missed bug before the release

🔥 🔥 🔥 🔥 🔥